refactor: use external file for storing task history #3481
Conversation
|
dosubot
bot
added
size:L
![ellipsis-dev[bot]](https://avatars.githubusercontent.com/in/64358?s=60&v=4){kind=small}
| async updateGlobalState<K extends GlobalStateKey>(key: K, value: GlobalState[K]) { | ||
| if (key === "taskHistory") { | ||
| this.stateCache[key] = value | ||
| if (Array.isArray(value)) { |
There was a problem hiding this comment.
In updateGlobalState, the new branch for taskHistory only proceeds if the value is an array. It would be helpful to add a type check (or log a warning) if a non-array is passed, to help catch unexpected types.
This comment was generated because it violated a code review rule: mrule_OR1S8PRRHcvbdFib.
| all: [], | ||
| }, | ||
| }, | ||
| FileSystemError: class { |
There was a problem hiding this comment.
Consider adding a code property to the FileSystemError mock so that error.code comparisons in ContextProxy.ts work as expected.
This comment was generated because it violated a code review rule: mrule_oAUXVfj5l9XxF01R.
| await proxy.initialize() | ||
|
|
||
| // Directly modify the private stateCache property to clear any task history | ||
| ;(proxy as any).stateCache.taskHistory = undefined |
There was a problem hiding this comment.
Directly modifying the private stateCache in tests (using ts-ignore) can be brittle. Consider exposing a test helper or method to reset state without bypassing encapsulation.
This comment was generated because it violated a code review rule: mrule_htmKIslKUiIk1Afo.
|
@elianiva you linked to a group of tasks. Does this fix all of those? |
|
What do you think about having a separate task history manager that handles all task history interactions (similar to the provider settings and custom modes managers), and we can remove it entirely from the If we go that route we might also want to create a base class for our various persistence-related managers (I can help with that). |
not all of them, only the ones related with the massive global state just to notify that there is a fix for some of them and people can confirm if it works or not
you know what, that's actually a better idea. bunching them all together is a bit of a pain to test and not really good to read want me to do it on another PR or is it fine if i just push more stuff on this one? |
|
@elianiva i will back this one off to draft and then you can push more stuff to this one. |
hannesrudolph
moved this from PR [Draft/WIP]
to PR [Pre Approval Review]
in Roo Code Roadmap
hannesrudolph
moved this from PR [Pre Approval Review]
to PR [Greenlit]
in Roo Code Roadmap
|
superseded by #3785 |
github-project-automation
bot
moved this from PR [Greenlit]
to Done
in Roo Code Roadmap
github-project-automation
bot
moved this from PR [Greenlit]
to Done
in Roo Code Roadmap
3 participants
Caution
CAUTION, DISCLAIMER, READ THIS
If you test this PR, you will lose your
taskHistoryfrom the global state because it gets moved to an external file. Please be aware before testing this branch.Related GitHub Issue
Not directly closes, but still related to the issue we've been having. See: #2391
This should address one of the causes
Description
Move task history out of globalState and use an external
taskHistory.jsonlto store the history. This way it can grow to whatever it needs to be without polluting the globalState option.I've added a mechanism to make the API reads from a file, so the code change should be minimal. I only need to update
ContextProxy.tsto do file operations specifically for thetaskHistorykey.Test Procedure
Upon opening vscode, you should see a little notification showing the task history has been migrated.
Type of Change
srcor test files.Pre-Submission Checklist
npm run lint).console.log) has been removed.npm test).mainbranch.npm run changesetif this PR includes user-facing changes or dependency updates.Screenshots / Videos
Other than this there's no screenshot as this is just moving where we store the history
Documentation Updates
Should we document this change? Like, telling where we store the history? I don't know
Additional Notes
Tests needed, still working on that
Important
Refactor task history management by moving it from global state to an external
taskHistory.jsonlfile, updatingContextProxy.tsand related tests.taskHistory.jsonlfile inContextProxy.ts.readTaskHistoryFile()andwriteTaskHistoryFile()handle file operations for task history.ContextProxy.test.tsto verify task history file operations.vscode.jsupdated to simulate file operations for task history.taskHistoryfromPASS_THROUGH_STATE_KEYSinContextProxy.ts.This description was created by
for b9dabdb. You can customize this summary. It will automatically update as commits are pushed.